Order enhancements #899

rlrossiter · 2017-11-30T22:17:55Z

This change adds enhancements to be able to use verifyOrder() and placeOrder() with keyNames that come from the API. There are also CLI enhancements to be able to do lookups to find packages, presets, categories, and items from the API, which are then fed into the verify/place_order functions.

This change adds a new command, and 5 new subcommands: slcli order - command for interacting with the ordering API slcli order category-list -- subcommand for listing categories in a package slcli order item-list -- subcommand for listing items in a package slcli order package-list -- subcommand for listing available packages slcli order place -- subcommand for verifying/placing orders slcli order preset-list -- subcommand for listing presets of a package API functions in SoftLayer.managers.ordering.OrderingManager were also added to programmatically interact with all of these functions as well. The place_order() and verify_order() commands are built to pass a package keyname, a location, and a list of item keynames. It then transforms the keynames into IDs, which the place/verifyOrder APIs accept.

The CLI appears to fail out if it gets AttributeError as part of any of the manager calls, so in order to not make it look like a softlayer-pythyon but, we need to spit out something from SoftLayer.exceptions. The base exceptions.SoftLayerError was used. Presets also needed to be handle differently. When listing the presets, both the activePresets and accountRestrictedActivePresets need to be retrieved and merged together before being returned. Finally, extra doc was added to verify_order() and place_order() to make it easier to understand how to use it.

coveralls · 2017-11-30T22:29:19Z

Coverage decreased (-1.2%) to 84.37% when pulling 67a54fd on rlrossiter:order-enhancements into 0062380 on softlayer:master.

I updated names from price_keynames -> item_keynames, but I missed a couple. Updated those to be consistent with item_keynames now.

coveralls · 2017-12-01T15:59:28Z

Coverage decreased (-1.2%) to 84.37% when pulling 9526a96 on rlrossiter:order-enhancements into 0062380 on softlayer:master.

rlrossiter · 2017-12-01T20:12:27Z

SoftLayer/managers/ordering.py

+    def get_preset_by_key(self, package_keyname, preset_keyname, mask=None):
+        """Gets a single preset with the given key."""
+        preset_operation = '_= %s' % preset_keyname
+        _filter = {'activePresets': {'keyName': {'operation': preset_operation}}}


This is only looking in the active presets. It also needs to look at the account restricted active presets with this filter to possibly find it in there as well

turns out this filter works with both activePresets and accountRestrictedActivePresets, so nothing needs to be updated.

coveralls · 2017-12-04T16:26:50Z

Coverage increased (+0.2%) to 85.812% when pulling 010f6f8 on rlrossiter:order-enhancements into 0062380 on softlayer:master.

coveralls · 2017-12-13T21:40:25Z

Coverage increased (+0.3%) to 85.854% when pulling dd62296 on rlrossiter:order-enhancements into 0062380 on softlayer:master.

coveralls · 2017-12-13T22:47:45Z

Coverage increased (+0.5%) to 86.069% when pulling 11ae799 on rlrossiter:order-enhancements into 0062380 on softlayer:master.

allmightyspiff · 2017-12-15T21:47:57Z

$ ./slcli --really order place   --complex-type SoftLayer_Container_Product_Order_Hardware_Server --preset D2690_128GB_2X600GBSAS_RAID1_2  --extras '{"hardware":[{"hostname":"testORdering", "domain": "cgallo.com"}]}' BARE_METAL_SERVER TORONTO  1_IP_ADDRESS UNLIMITED_SSL_VPN_USERS_1_PPTP_VPN_USER_PER_ACCOUNT REBOOT_KVM_OVER_IP OS_UBUNTU_16_04_LTS_XENIAL_XERUS_64_BIT BANDWIDTH_0_GB_2 100_MBPS_PRIVATE_NETWORK_UPLINK
id       21469963
created  2017-12-15T15:41:31-06:00
status   PENDING_AUTO_APPROVAL

Noticed some oddness with locations though, I'm not sure why DAL13 is an invalid location here. Using any of the 3 letter dc names doens't seem to work.

$ ./slcli --really order place   --complex-type SoftLayer_Container_Product_Order_Hardware_Server --preset D2690_128GB_2X600GBSAS_RAID1_2  --extras '{"hardware":[{"hostname":"testORdering", "domain": "cgallo.com"}]}' BARE_METAL_SERVER DAL13 1_IP_ADDRESS UNLIMITED_SSL_VPN_USERS_1_PPTP_VPN_USER_PER_ACCOUNT REBOOT_KVM_OVER_IP OS_UBUNTU_16_04_LTS_XENIAL_XERUS_64_BIT BANDWIDTH_0_GB_2 100_MBPS_PRIVATE_NETWORK_UPLINK
SoftLayerAPIError(SoftLayer_Exception_Order_InvalidLocation): The location provided for this order is invalid.

Using its keyname seems to work though. Adding in a way to get the available DCs for a package might be nice.

$ ./slcli --really order place   --complex-type SoftLayer_Container_Product_Order_Hardware_Server --preset D2690_128GB_2X600GBSAS_RAID1_2  --extras '{"hardware":[{"hostname":"testORdering", "domain": "cgallo.com"}]}' BARE_METAL_SERVER DALLAS13 1_IP_ADDRESS UNLIMITED_SSL_VPN_USERS_1_PPTP_VPN_USER_PER_ACCOUNT REBOOT_KVM_OVER_IP OS_UBUNTU_16_04_LTS_XENIAL_XERUS_64_BIT BANDWIDTH_0_GB_2 100_MBPS_PRIVATE_NETWORK_UPLINK
SoftLayerAPIError(SoftLayer_Exception_Order_HardwareUnavailable): There is currently no hardware available for package # 200 with preset configuration # 101 in Dallas 13. Please select a different datacenter

allmightyspiff

Let me know your thoughts on the complexType thing and location comment. Even with those I'd be ready to merge this monday unless you want to make changes.

allmightyspiff · 2017-12-15T22:10:28Z

SoftLayer/order_utils.py

+"""Utility functions for mapping packages and categories to complex types."""
+
+
+def get_package_type_map():


would it make sense to add the virtual server and bare metal server types here?

Apparently what I've been told, we can't map directly from that package to that order type

allmightyspiff · 2017-12-15T22:13:42Z

SoftLayer/order_utils.py

+    }
+
+
+def get_category_type_map():


I'm not sure how I feel about this level of hard coding categories.
Having the user figure out the complexType on their own might be preferable here. Thoughts?

Yeah, my goal was to just make this as simple as possible for the user, because it's not obviously clear where to find the SoftLayer_Container_Product_Order type that needs to be used. Unfortunately, this was the only way to do that (there is no API call to properly get the type).

Maybe I should just move this out into a different PR that we can debate on (it'll thankfully make this change a little bit smaller). I'll make complexType a kwarg and CLI option so one day we'll be able to make it optional, but for now I'll require it to be set. How does that sound?

Yeah thats a good plan.

rlrossiter · 2017-12-18T14:51:27Z

Noticed some oddness with locations though, I'm not sure why DAL13 is an invalid location here. Using any of the 3 letter dc names doens't seem to work.

So the order API requires you to specify the full name for orders when you provide a location. I was thinking of doing a follow-up PR later to allow you to specify the short names for DCs as well.

rlrossiter · 2017-12-19T21:24:37Z

SoftLayer/managers/ordering.py

+        """
+        # verify the order, and if the order is valid, the proper prices will be filled
+        # into the order template, so we can just send that to placeOrder to order it
+        verified_order = self.verify_order(package_keyname, location, item_keynames,


verifyOrder() can strip data out of the original order in the returned order, so calling placeOrder with the verified order will be omitting some of the original order data. verify_order() and place_order() should then have a same base prepare_order() function that it then either calls verify or place on, depending on the function it is in.

allmightyspiff

unapproving for now based on our conversation.

verifyOrder and placeOrder need to be called with the same generated order dict, and the output of verifyOrder can't be sent to placeOrder, since it strips out some extra data.

coveralls · 2018-01-02T15:07:14Z

Coverage increased (+0.2%) to 86.55% when pulling 9d6c962 on rlrossiter:order-enhancements into 5f16b02 on softlayer:master.

rlrossiter · 2018-01-02T20:51:05Z

@allmightyspiff I think this is now in a final working state if you want to take a look at it. Thanks!

allmightyspiff

looks good. I'll likely merge later this week

Ryan Rossiter added 2 commits November 22, 2017 14:37

Fix more item_keyname renames

9526a96

I updated names from price_keynames -> item_keynames, but I missed a couple. Updated those to be consistent with item_keynames now.

rlrossiter commented Dec 1, 2017

View reviewed changes

allmightyspiff self-requested a review December 1, 2017 20:15

Add unit tests for the order manager and CLI

010f6f8

allmightyspiff approved these changes Dec 15, 2017

View reviewed changes

rlrossiter commented Dec 19, 2017

View reviewed changes

allmightyspiff requested changes Dec 19, 2017

View reviewed changes

Ryan Rossiter added 2 commits December 19, 2017 16:38

Add CLI doc and move order generation

08255fe

verifyOrder and placeOrder need to be called with the same generated order dict, and the output of verifyOrder can't be sent to placeOrder, since it strips out some extra data.

Fix up generate_order() unit tests

9d6c962

allmightyspiff approved these changes Jan 2, 2018

View reviewed changes

allmightyspiff merged commit a1e984a into softlayer:master Jan 5, 2018

		"""Utility functions for mapping packages and categories to complex types."""


		def get_package_type_map():

Order enhancements #899

Order enhancements #899

Uh oh!

Conversation

rlrossiter commented Nov 30, 2017

Uh oh!

coveralls commented Nov 30, 2017

Uh oh!

coveralls commented Dec 1, 2017

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

coveralls commented Dec 4, 2017

Uh oh!

coveralls commented Dec 13, 2017

Uh oh!

coveralls commented Dec 13, 2017

Uh oh!

allmightyspiff commented Dec 15, 2017

Uh oh!

allmightyspiff left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

rlrossiter commented Dec 18, 2017

Uh oh!

Choose a reason for hiding this comment

Uh oh!

allmightyspiff left a comment

Choose a reason for hiding this comment

Uh oh!

coveralls commented Jan 2, 2018

Uh oh!

rlrossiter commented Jan 2, 2018

Uh oh!

allmightyspiff left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!